perf(storer): reuse buffers and eliminate per-chunk allocations in reserve sample by gacevicljubisa · Pull Request #5612 · ethersphere/bee

gacevicljubisa · 2026-09-14T12:06:49Z

Checklist

I have read the coding guide.
My change requires a documentation update, and I have done it.
I have added tests to cover my changes.
I have filled out the description and linked the related issues.

Description

This PR addresses per-chunk allocations and buffer overhead in ReserveSample (issue #5174):

GetterInto interface: Adds storage.GetterInto to allow reading chunk data directly into a caller-provided reusable buffer, avoiding per-call buffer allocations in storage readers.
Hoist ChunkStore handle: Instantiates ChunkStore() once per worker instead of per chunk, eliminating 3 allocations per chunk.
Per-worker reusable buffer: Each sampler worker reuses a swarm.SocMaxChunkSize buffer for chunk reading and feeds raw byte slices directly into transformedAddress, eliminating swarm.NewChunk heap
escapes.
Lazy chunk loading for candidates: Full chunk payloads are no longer pushed through sampleItemChan for all millions of chunks. Full chunk data is loaded in Phase 3 only for the ~200 items that
actually qualify for the sample candidate set.
Raw batch ID transfer: Avoids instantiating postage.NewStamp for every iterated chunk in workers by passing raw batchID through an internal candidate struct.
Safety guards & benchmarks: Adds assertSampleDataIntact and assertSampleDataNotShared to prevent buffer aliasing/corruption regressions, adds BenchmarkChunkStoreGet micro-benchmark, and adds
BenchmarkReserveSample10k.

A/B on bee-light-testnet, two nodes with identical config (no SIMD, verbosity 3, 1500m CPU, storage radius 2, isWarmingUp: false). Sample triggered via GET /rchash/2/<own-overlay>/<anchor2>; allocations attributed with pprof -diff_base focused on the ReserveSample subtree.

Per chunk (ReserveSample subtree)	master `b6537ea2`	this branch	Δ
Allocations	132.7	104.0	−21.7%
↳ chunk retrieval	60.5	36.4	−39.9%
Bytes	11,007 B	4,489 B	−59.2%
↳ chunk retrieval	8,774 B	2,615 B	−70.2%
↳ `chunkstore.readChunk` buffer	4,876 B	0 B	−100%
`transformedAddress` (control)	68.3	67.6	−1.1%
`bmt.Write` (control)	63.0	62.6	−0.7%

Total allocation churn per sample round: 24.65 GB → 10.30 GB over 2.405M chunks.

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Screenshots (if appropriate):

AI Disclosure

This PR contains code that has been generated by an LLM.
I have reviewed the AI generated code thoroughly.
I possess the technical expertise to responsibly review the code generated in this PR.

Sampling is about to start reading chunks into a buffer that each worker reuses. Nothing today checks that the bytes handed back in a SampleItem are still the bytes of that chunk, so a reused buffer would silently hand the redistribution proof the contents of some later chunk. assertValidSample now checks two things for every item: that ChunkData still reproduces ChunkAddress, and that no two items share a backing array. Every existing sample test picks both up. Also add the rulers for the work that follows. BenchmarkReserveSample1k keeps its name and behaviour so the recorded baseline stays comparable; its body moves to a helper that BenchmarkReserveSample10k reuses over a ten times larger reserve. BenchmarkChunkStoreGet measures a single chunk read, split into a variant that builds the ChunkStore handle per call as the sampler does today and one that hoists it, so the cost of the handle alone is visible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq

db.ChunkStore() builds three objects every time it is called, and the sampler called it once per chunk. On a testnet node that is 2.3 million handles per round, thrown away immediately. Hoist it to one per worker. The handle is deliberately not shared across workers: the read-only chunk store makes no thread-safety promise, and three allocations per worker is already nothing next to three per chunk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq

…lazily

…locations

Both ReserveSample benchmarks fill their reserve with chunk.GenerateValidRandomChunkAt, which produces content-addressed chunks only. The SOC branch of transformedAddress is therefore never measured by them, and any work that branch does beyond hashing the wrapped CAC is invisible. Benchmark the function directly, one case per chunk type, through a shim in export_test.go. The shim takes a swarm.Chunk and is held to that signature on purpose so the same benchmark source can be run against branches whose internal transformedAddress is shaped differently; only the shim changes between them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq

aloknerurkar · 2026-09-15T18:53:34Z


 type ChunkStore interface {
 	Getter
+	GetterInto


Not sure if we need to add it to the ChunkStore interface. Since we only use ReadOnlyChunkStore for sampling, we can only add it to the interface below. I think we can even avoid adding it to transaction.ChunkStore also.

Dropping it makes ReadOnlyChunkStore no longer a subset of ChunkStore, which breaks the test storages and the mock storer that return the same value as both. Then even more changes are needed...

aloknerurkar · 2026-09-15T18:54:04Z

 	return ch, err
 }

+func (c *chunkStoreTrx) GetInto(ctx context.Context, addr swarm.Address, buf []byte) (n int, err error) {


Check comment above. Since we only need ReadOnlyChunkStore to provide this function, we can avoid adding it here.

martinconic · 2026-09-18T12:33:24Z

+				continue
+			}
+
+			ch, err := phase3ChunkStore.Get(ctx, item.chunkAddress)


instead ctx shouldn't we also use in this Phase 3 gCtx ?

martinconic · 2026-09-18T13:30:18Z

+				continue
+			}
+
+			ch, err := phase3ChunkStore.Get(ctx, item.chunkAddress)


phase3ChunkStore.Get runs before the consensusTime check: stamp.Timestamp() is already in memory from Step 1. Loading the 4096-byte chunk from Sharky before verifying stamp.Timestamp() <= consensusTime causes completely wasted disk reads for every chunk stamped past consensusTime.

Also, phase3ChunkStore.Get runs before db.validStamp: In Swarm, ValidStamp does not inspect chunk data—it only validates chunk.Address() and chunk.Stamp(). Performing a disk read before verifying whether the stamp is even valid burns I/O for expired or invalid stamps.

martinconic · 2026-09-18T14:04:35Z

 // state and concurrent calls would see flapping values. On platforms where the
 // dispatcher falls back to the goroutine pool (non-linux/amd64 or CPU without
 // AVX2/AVX-512), the SIMD sub-test degrades to a goroutine run.
 func TestSampleVectorCAC(t *testing.T) {


Should we also have a TestSampleVectorSOC, because changes were made in that area?

martinconic · 2026-09-18T14:07:04Z


 type ChunkStore interface {
 	Getter
+	GetterInto


GetterInto was added to the required ChunkStore/ReadOnlyChunkStore interfaces, and forgetting.go embeds storage.ChunkStore. DelayedStore and ForgettingStore override Get/Put but not GetInto, so the promoted method forwards straight through — a chunk marked Miss() or Delay()ed comes back normally. Latent today, but it'll bite whoever first routes a read path through GetInto.

martinconic · 2026-09-18T14:11:05Z

@@ -21,6 +21,14 @@ type Getter interface {
 	Get(context.Context, swarm.Address) (swarm.Chunk, error)
 }



implementations return a bare fmt.Errorf("chunk store: buffer too small: %d < %d", ...). A caller can't distinguish it from ErrNotFound.
Maybe add:

// ErrBufferTooSmall is returned by GetInto when len(buf) is smaller than the // stored chunk. Callers may inspect it with errors.Is to retry with a larger buffer. var ErrBufferTooSmall = errors.New("storage: buffer too small")

gacevicljubisa and others added 2 commits September 14, 2026 11:57

github-actions Bot assigned gacevicljubisa Sep 14, 2026

gacevicljubisa added the testing label Sep 14, 2026

gacevicljubisa added 2 commits September 14, 2026 16:02

perf(storer): read into reusable worker buffer and load sample items …

1682441

…lazily

perf(storer): pass batch id through sampler channel to avoid stamp al…

841bbaf

…locations

gacevicljubisa force-pushed the perf/sample-01-hoist-chunkstore branch from 444d89d to 841bbaf Compare September 14, 2026 14:11

gacevicljubisa requested review from acud and janos September 15, 2026 14:16

gacevicljubisa removed the testing label Sep 15, 2026

acud reviewed Sep 15, 2026

View reviewed changes

Comment thread pkg/file/joiner/joiner_test.go

aloknerurkar reviewed Sep 15, 2026

View reviewed changes

fix(storage): check buffer capacity in GetInto

207b420

gacevicljubisa requested a review from acud September 15, 2026 21:00

acud reviewed Sep 15, 2026

View reviewed changes

Comment thread pkg/storer/sample.go

gacevicljubisa requested review from akrem-chabchoub, martinconic and sbackend123 September 16, 2026 14:21

fix(storage): check buffer length in GetInto, not capacity

b9bf777

martinconic reviewed Sep 18, 2026

View reviewed changes

Navigation Menu

perf(storer): reuse buffers and eliminate per-chunk allocations in reserve sample - #5612

perf(storer): reuse buffers and eliminate per-chunk allocations in reserve sample#5612
gacevicljubisa wants to merge 7 commits into
masterfrom
perf/sample-01-hoist-chunkstore

gacevicljubisa commented Sep 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

aloknerurkar Sep 15, 2026

Uh oh!

gacevicljubisa Sep 15, 2026

Uh oh!

aloknerurkar Sep 15, 2026

Uh oh!

Uh oh!

martinconic Sep 18, 2026

Uh oh!

martinconic Sep 18, 2026

Uh oh!

martinconic Sep 18, 2026

Uh oh!

martinconic Sep 18, 2026

Uh oh!

martinconic Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -21,6 +21,14 @@ type Getter interface {
		Get(context.Context, swarm.Address) (swarm.Chunk, error)
		}

Conversation

gacevicljubisa commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Description

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Screenshots (if appropriate):

AI Disclosure

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gacevicljubisa commented Sep 14, 2026 •

edited

Loading